-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix vis editor layout #53292
Fix vis editor layout #53292
Conversation
Pinging @elastic/kibana-app (Team:KibanaApp) |
width: 100%; | ||
height: 100%; | ||
} | ||
height: 100%; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When exaclty was the regression introduced?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @majagrubic !
I've found the root cause and updated the PR description
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix LGTM in Chrome, @sulemanof Can you be sure to check in IE11?
The current version of kibana is not supported in IE11: Checked in Firefox & Edge, works fine: |
@sulemanof It's still supported, you have to set |
@flash1293 thanks for the hint! But I still can't run kibana in IE, it is stuck loading... |
Seems like there is something else off, I will look into it. |
Summary
REGRESSION. Caused by #49891.
The reason is caused by wrapping the layout with additional
div
s here: https://github.com/elastic/kibana/pull/49891/files#diff-1ca3346c218d8218751dbe59736eb72aR66The default vis editor layout doesn't take all the window height:
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers